gh-149306: Improve error messages in wave module to include the offending value#149307
gh-149306: Improve error messages in wave module to include the offending value#149307htjworld wants to merge 3 commits intopython:mainfrom
Conversation
… offending value Include the offending value in wave.Error messages raised by Wave_read._read_fmt_chunk and Wave_write setters (setnchannels, setsampwidth, setframerate). For setframerate, the original value is shown rather than the rounded one to aid debugging.
htjworld
force-pushed
the
gh-149306-wave-error-messages
branch
2 times, most recently
from
cc28830 to
d1ba708
Compare
…e-error-messages
| def _read_fmt_chunk(self, chunk): | ||
| try: | ||
| self._format, self._nchannels, self._framerate, dwAvgBytesPerSec, wBlockAlign = struct.unpack_from('<HHLLH', chunk.read(14)) | ||
| self._format, nchannels, self._framerate, dwAvgBytesPerSec, wBlockAlign = struct.unpack_from('<HHLLH', chunk.read(14)) |
There was a problem hiding this comment.
Don't change this. It is a breaking change. Previously the attribute would have been already set after passing unpack_from but now it will be deferred.
| rounded = int(round(framerate)) | ||
| if rounded <= 0: | ||
| raise Error(f'bad frame rate: {framerate!r}') | ||
| self._framerate = rounded |
There was a problem hiding this comment.
| rounded = int(round(framerate)) | |
| if rounded <= 0: | |
| raise Error(f'bad frame rate: {framerate!r}') | |
| self._framerate = rounded | |
| rounded_framerate = int(round(framerate)) | |
| if rounded_framerate <= 0: | |
| raise Error(f'bad frame rate: {framerate!r}') | |
| self._framerate = rounded_framerate |
| with self.assertRaisesRegex(wave.Error, | ||
| re.escape(f'bad # of channels: {nchannels!r}')): |
There was a problem hiding this comment.
For readability purposes, can you put the message outside?
message = re.escape(...)
with wave.open(...):
with self.assertRaisesRegex(..., err):
f.setchannels(channels)
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
…te, extract test messages
|
I have made the requested changes; please review again
|
|
Thanks for making the requested changes! @picnixz: please review the changes made to this pull request. |
Documentation build overview
8 files changed ·
|
2 participants
Summary
Include the offending value in
wave.Errormessages raised by:Wave_read._read_fmt_chunk(parses WAV file header)Wave_write.setnchannelsWave_write.setsampwidthWave_write.setframerateBefore:
After:
Motivation
Wave_read: Values come from the WAV file header — they never appearin the traceback. The error message is the only debugging clue.
Wave_write: When values originate from variables, configuration files,or parsed data rather than literals, the traceback shows the call site but
not the value itself. For example,
w.setframerate(config['rate'])raisesbad frame ratewith no indication of whatconfig['rate']was.This extends the clear-feedback approach introduced in gh-144976, which fixed
the validation order in
setframerateto raise immediately at the point ofthe mistake. Showing the offending value is the natural next step.
For
setframerate, the original value (beforeint(round())) is shown ratherthan the rounded result, so callers see exactly what they passed.
Precedent
statistics:raise StatisticsError(f'No negative inputs allowed: {x!r}')(statistics: Fix geometric_mean() error message for negative inputs #149246)asyncio:raise ValueError(f'A Stream Socket was expected, got {sock!r}')Notes
bad sample widthpredates this PR and can be addressed separately.NaN/Infhandling insetframerateis pre-existing behaviour and out of scope here.